Skip to content

add option to put docstrings on model attributes #1190

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 10 commits into from
Mar 3, 2025

Conversation

eli-bl
Copy link
Collaborator

@eli-bl eli-bl commented Jan 7, 2025

Adds a docstrings_on_attributes option that causes property descriptions to appear in docstrings on the attributes, instead of in the class docstring. This is a desirable behavior when using documentation generators like Sphinx.

This is a non-breaking change; if the option is not set, code generation is exactly the same as before (evidence: the existing golden-record directories have no changes).

Clarification about how docstrings on attributes work

Python does not treat all docstrings the same, in terms of availability at runtime. A docstring on a class or method is available as a __doc__ attribute— but a docstring on an attribute is not; such a string is just thrown away by the interpreter. If class A has attribute b, then A.b.__doc__ will never be a thing— you would have to define b as a property getter method instead.

However, docstrings on attributes are still valid as per PEP 257 as a thing that can be surfaced by other tools. Sphinx will use them when generating documentation, and VS Code will show them as a description in the popup if you hover over an attribute.

@eli-bl eli-bl requested a review from dbanty January 7, 2025 18:14
Copy link
Collaborator

@dbanty dbanty left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you use a smaller OpenAPI document for the snapshots on this one so there will be fewer changes in the future as we tweak things?

Probably just a single endpoint & model would be enough to demonstrate it

@eli-bl eli-bl force-pushed the model-attr-docstrings branch from ec3a812 to 0c73307 Compare January 7, 2025 20:36
@eli-bl eli-bl requested a review from dbanty January 7, 2025 22:05
@eli-bl
Copy link
Collaborator Author

eli-bl commented Jan 9, 2025

@dbanty FYI, this was motivated partly by trying to build some HTML docs from a generated client with Sphinx, and finding that Sphinx really does not like how the current class docstrings are formatted. The format only really works if you view the strings as plain text; if it's interpreted either as Sphinx's default .rst format, or as Markdown, it becomes very garbled. That's the case even if the class & property descriptions are just plain text with no special markup - it's just a consequence of how indents & line breaks are being used in the template. Regardless of what kind of markup is supported by any particular tool, they are all likely to have opinions about those things, so it's kind of asking for trouble to try to create structured text like this out of multiple pieces of data. By contrast, the Python code structure, and the idea of class docstrings vs. attribute docstrings, is already something doc generators and IDEs understand.

@eli-bl
Copy link
Collaborator Author

eli-bl commented Jan 10, 2025

@dbanty Btw, I updated the PR description to clarify the behavior of attribute docstrings in Python.

@eli-bl eli-bl force-pushed the model-attr-docstrings branch from 80c799d to e980228 Compare January 13, 2025 19:55
@eli-bl eli-bl force-pushed the model-attr-docstrings branch from e980228 to 7c3427e Compare January 13, 2025 19:59
@eli-bl eli-bl requested a review from dbanty January 13, 2025 20:02
@dbanty dbanty added this pull request to the merge queue Mar 3, 2025
Merged via the queue into openapi-generators:main with commit 44b1fbc Mar 3, 2025
22 checks passed
micha91 pushed a commit to micha91/openapi-python-client that referenced this pull request May 13, 2025
…1190)

Adds a `docstrings_on_attributes` option that causes property
descriptions to appear in docstrings on the attributes, instead of in
the class docstring. This is a desirable behavior when using
documentation generators like Sphinx.

This is a non-breaking change; if the option is not set, code generation
is exactly the same as before (evidence: the existing golden-record
directories have no changes).

## Clarification about how docstrings on attributes work

Python does not treat all docstrings the same, in terms of availability
at runtime. A docstring on a class or method is available as a `__doc__`
attribute— but a docstring on an attribute is not; such a string is just
thrown away by the interpreter. If class `A` has attribute `b`, then
`A.b.__doc__` will never be a thing— you would have to define `b` as a
property getter method instead.

However, docstrings on attributes are still valid as per [PEP
257](https://peps.python.org/pep-0257/) as a thing that can be surfaced
by other tools. Sphinx _will_ use them when generating documentation,
and VS Code _will_ show them as a description in the popup if you hover
over an attribute.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants